Change SkipStatusCodePagesAttribute to inherit from IAlwaysRunResultFilter #10490
Conversation
|
|
Eilon
added
the
area-mvc
huan086
force-pushed
the
feature/always-run-SkipStatusCodePagesAttribute
branch
from
336e1d3 to
f94334c
Compare
…ilter instead of IResourceFilter (fixes dotnet#10317).
…n ensure it executes first (dotnet#10317).
huan086
force-pushed
the
feature/always-run-SkipStatusCodePagesAttribute
branch
from
f94334c to
6f08099
Compare
| /// </summary> | ||
| [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] | ||
| public class SkipStatusCodePagesAttribute : Attribute, IResourceFilter | ||
| public class SkipStatusCodePagesAttribute : Attribute, IAlwaysRunResultFilter, IOrderedFilter |
There was a problem hiding this comment.
While I think this change makes sense, this does
a) is a breaking change
b) does not work with endpoint routing
We should leave the IResourceFilter on there since it does not hurt to invoke the filter in multiple stages and allows it to be non-breaking. In addition, as of 3.0, Auth happens as part of the middleware pipeline, so this change really doesn't help there. If you'd like to solve it for the latter, one solution would be to:
a) Introduce an ISkipStatusCodePagesMetadata
b) Use the presence of the metadata to skip the status code page if you're doing endpoint routing. Here's a similar pattern: https://github.com/aspnet/AspNetCore/blob/master/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs#L140-L159
|
Cool, let me revert the changes and follow the way CorsMiddleware does it |
|
I haven't have the time to make the necessary changes yet |
|
Feel free to re-send this PR once you've had a chance to update it per PR comments. |
5 participants
Summary of the changes
Addresses #10317